Skip to content

Align mobile SDK size recommendation API requests with web implementation#165

Merged
mikhail-yevstratiev merged 6 commits into
trunkfrom
fix/implement-relevant-size-recommendation-API-requests
Aug 4, 2025
Merged

Align mobile SDK size recommendation API requests with web implementation#165
mikhail-yevstratiev merged 6 commits into
trunkfrom
fix/implement-relevant-size-recommendation-API-requests

Conversation

@mikhail-yevstratiev

Copy link
Copy Markdown
Contributor

🔗 Related Links

⬅️ As Is

Explain the current situation of the code

Android SDK uses outdated size recommendation API requests

➡️ To Be

  • Android SDK size recommendation API requests are aligned with web implementation

☑️ Checklist

  • Useless comments and/or Log.d etc are removed
  • Unit test are covered
  • Code has been deployed and tested on STG
  • ClickUp ticket status has been updated to production
  • Update CHANGELOG.md with the new changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR aligns the Android SDK's size recommendation API requests with the web implementation by updating parameter naming conventions and adding support for shoe size recommendations.

Key changes:

  • Renamed API methods to distinguish between item and shoe size recommendations
  • Updated API parameter names from camelCase to snake_case to match web standards
  • Added footwear data support to user body profiles

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
VirtusizeAPIService.kt Renamed method and added new shoe size recommendation API method
VirtusizeAPIServiceImpl.kt Implemented separate endpoints for item and shoe size recommendations
BodyProfileRecommendedSizeParams.kt Updated parameter naming to snake_case and added shoe-specific parameter mapping
UserBodyProfile.kt Added footwear data field to support shoe recommendations
VirtusizeApi.kt Added new API endpoint for shoe size recommendations
Test files Updated tests to reflect new parameter naming and added footwear data assertions

Comment on lines +266 to +267
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug code should be removed from production. The line creating jsn variable and the print(jsn) statement appear to be debugging code that should not be included in the final implementation.

Suggested change
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot uses AI. Check for mistakes.
withContext(Dispatchers.IO) {
val apiRequest = VirtusizeApi.getSize(productTypes, storeProduct, userBodyProfile)
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug code should be removed from production. This print statement appears to be debugging code that should not be included in the final implementation.

Suggested change
print(jsn);

Copilot uses AI. Check for mistakes.
Comment on lines +284 to +285
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug code should be removed from production. The line creating jsn variable and the print(jsn) statement appear to be debugging code that should not be included in the final implementation.

Suggested change
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot uses AI. Check for mistakes.
withContext(Dispatchers.IO) {
val apiRequest = VirtusizeApi.getShoeSize(productTypes, storeProduct, userBodyProfile)
val jsn = JSONObject(apiRequest.params).toString();
print(jsn);

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug code should be removed from production. This print statement appears to be debugging code that should not be included in the final implementation.

Suggested change
print(jsn);

Copilot uses AI. Check for mistakes.
userBodyProfileResponse.successData!!,
)
return bodyProfileRecommendedSizeResponse.successData?.get(0)?.sizeName
if (storeProduct.productType == 17) {

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number 17 should be replaced with a named constant or enum value to improve code readability and maintainability. Consider defining a constant like PRODUCT_TYPE_SHOES = 17 or using an enum.

Suggested change
if (storeProduct.productType == 17) {
if (storeProduct.productType == PRODUCT_TYPE_SHOES_ID) {

Copilot uses AI. Check for mistakes.
"toeShape" to "greek",
"size" to "30.5",
"type" to "sneakers",
"brand" to "virtusize",

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brand name inconsistency: 'virtusize' should be capitalized as 'Virtusize' to match the brand name used elsewhere in the codebase (line 99 shows 'Virtusize').

Suggested change
"brand" to "virtusize",
"brand" to "Virtusize",

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikhail-yevstratiev please update the brand name

fun paramsToMap(): Map<String, Any> {
return emptyMap<String, Any>()
.plus(
mapOf("app_origin" to 2),

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Magic number 2 should be replaced with a named constant to improve code readability and maintainability. Consider defining a constant like APP_ORIGIN_ANDROID = 2.

Suggested change
mapOf("app_origin" to 2),
mapOf("app_origin" to APP_ORIGIN_ANDROID),

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikhail-yevstratiev resolve the comment

@devAbhimanyu devAbhimanyu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments

"toeShape" to "greek",
"size" to "30.5",
"type" to "sneakers",
"brand" to "virtusize",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikhail-yevstratiev please update the brand name

* @param userBodyProfile [UserBodyProfile]
* @see ApiRequest
*/
fun getShoeSize(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getShoeSize -> getShoeSizeRecommendation

fun paramsToMap(): Map<String, Any> {
return emptyMap<String, Any>()
.plus(
mapOf("app_origin" to 2),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikhail-yevstratiev resolve the comment

@devAbhimanyu devAbhimanyu left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mikhail-yevstratiev mikhail-yevstratiev merged commit 3b62afa into trunk Aug 4, 2025
3 checks passed
@mikhail-yevstratiev mikhail-yevstratiev deleted the fix/implement-relevant-size-recommendation-API-requests branch August 4, 2025 07:26
@mikhail-yevstratiev mikhail-yevstratiev mentioned this pull request Aug 4, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants